-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
azurerm_mssql_elasticpool - Fix for creating hyperscale elastic pool of type MOPRMS and PRMS #28453
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR @mkohn. I'm not able to reproduce the problem described in the issue that these changes are supposed to fix.
Looking through our PR history it appears #28178 should have closed #27908 and went out in v4.13.0 of the provider.
We're happy to add support for additional sku's though, would you mind resolving the comments I left in-line on that?
4: 128000, | ||
6: 128000, | ||
8: 128000, | ||
10: 128000, | ||
12: 128000, | ||
14: 128000, | ||
16: 128000, | ||
18: 128000, | ||
20: 128000, | ||
24: 128000, | ||
32: 128000, | ||
40: 128000, | ||
80: 128000, | ||
}, | ||
"prms": { | ||
4: 1024, | ||
6: 1536, | ||
8: 2048, | ||
10: 2048, | ||
12: 3072, | ||
14: 3072, | ||
16: 3072, | ||
18: 3072, | ||
20: 3072, | ||
24: 4096, | ||
32: 4096, | ||
40: 4096, | ||
80: 4096, | ||
4: 128000, | ||
6: 128000, | ||
8: 128000, | ||
10: 128000, | ||
12: 128000, | ||
14: 128000, | ||
16: 128000, | ||
18: 128000, | ||
20: 128000, | ||
24: 128000, | ||
32: 128000, | ||
40: 128000, | ||
64: 128000, | ||
80: 128000, | ||
128: 128000, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe these should be changed to some arbitrary maximum value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stephybun They are not set to arbitrary maximum value. They are set to 128TB see https://learn.microsoft.com/en-us/azure/azure-sql/database/resource-limits-vcore-single-databases?view=azuresql#hyperscale---provisioned-compute---gen5 for more details.
4: 128000, | ||
6: 128000, | ||
8: 128000, | ||
10: 128000, | ||
12: 128000, | ||
14: 128000, | ||
16: 128000, | ||
18: 128000, | ||
20: 128000, | ||
24: 128000, | ||
32: 128000, | ||
40: 128000, | ||
64: 128000, | ||
80: 128000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, we should be setting meaningful values here. The Azure documentation will probably have some information on what the values of these should be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stephybun They are not set to arbitrary maximum value. They are set to 128TB see https://learn.microsoft.com/en-us/azure/azure-sql/database/resource-limits-vcore-single-databases?view=azuresql#hyperscale---provisioned-compute---gen5 for more details. and for the premium memory optimized https://learn.microsoft.com/en-us/azure/azure-sql/database/resource-limits-vcore-single-databases?view=azuresql#hyperscale-premium-series-part-1-of-3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the supporting links @mkohn LGTM 👍
* Update for #27950 * Update for #27824 * Update for #28592 #28583 #28599 #28590 #28453 * Update #28528 * Update for #27853 * Update CHANGELOG.md #28221 * Update for #27760 * Update CHANGELOG.md #28480 * Update CHANGELOG.md typo * Update CHANGELOG.md #28372 * Update CHANGELOG.md for #26047 also alphabetised ENHANCEMENTS * Update for #28146 * Update CHANGELOG.md #28013 * Update CHANGELOG.md for #28492 * Update CHANGELOG.md for #28648 * Update CHANGELOG.md for #28549 * Update for #28469 #28620 * prep for release * i touched it last --------- Co-authored-by: catriona-m <[email protected]> Co-authored-by: jackofallops <[email protected]> Co-authored-by: sreallymatt <[email protected]> Co-authored-by: Matthew Frahry <[email protected]> Co-authored-by: jackofallops <[email protected]>
* Update for hashicorp#27950 * Update for hashicorp#27824 * Update for hashicorp#28592 hashicorp#28583 hashicorp#28599 hashicorp#28590 hashicorp#28453 * Update hashicorp#28528 * Update for hashicorp#27853 * Update CHANGELOG.md hashicorp#28221 * Update for hashicorp#27760 * Update CHANGELOG.md hashicorp#28480 * Update CHANGELOG.md typo * Update CHANGELOG.md hashicorp#28372 * Update CHANGELOG.md for hashicorp#26047 also alphabetised ENHANCEMENTS * Update for hashicorp#28146 * Update CHANGELOG.md hashicorp#28013 * Update CHANGELOG.md for hashicorp#28492 * Update CHANGELOG.md for hashicorp#28648 * Update CHANGELOG.md for hashicorp#28549 * Update for hashicorp#28469 hashicorp#28620 * prep for release * i touched it last --------- Co-authored-by: catriona-m <[email protected]> Co-authored-by: jackofallops <[email protected]> Co-authored-by: sreallymatt <[email protected]> Co-authored-by: Matthew Frahry <[email protected]> Co-authored-by: jackofallops <[email protected]>
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Community Note
Description
This PR adds the ability to leverage Memory Optimized Premium
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)
module.hyperscale1.azurerm_mssql_elasticpool.sql will be updated in-place
~ resource "azurerm_mssql_elasticpool" "sql" {
id = "/subscriptions/xxxxxxxxx/resourceGroups/xxxxx-xxxx-xxxx/providers/Microsoft.Sql/servers/xxxxxxx/elasticPools/xxxx-xxx-xxxx-Prm1"
name = "xxxxxx-xxxxx-xxxxx-Prm1"
# (8 unchanged attributes hidden)
}
Apply complete! Resources: 0 added, 1 changed, 0 destroyed.
For state migrations please test the changes locally and provide details here, such as the versions involved in testing the migration path. For further details on testing state migration changes please see our guide on state migrations in the contributor documentation. -->
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_mssql_elasticpool
- adding support for the Sku MOPRMS and PRMSThis is a (please select all that apply):
Related Issue(s)
Fixes #27908
Note
If this PR changes meaningfully during the course of review please update the title and description as required.